fix: ensure FlutterResult and invokeMethod are always called on main thread#98
Open
muzahidul-opti wants to merge 5 commits intomasterfrom
Open
fix: ensure FlutterResult and invokeMethod are always called on main thread#98muzahidul-opti wants to merge 5 commits intomasterfrom
muzahidul-opti wants to merge 5 commits intomasterfrom
Conversation
…thread
iOS 16 requires FlutterResult to be invoked on the main thread. When
OptimizelyClient.start() or decideAsync closures call result() from a
background thread under multi-SDK startup contention, iOS 16 silently
drops the response causing the Dart Future to never resolve or return
null, which previously crashed the app with an unhandled TypeError or
PlatformException.
Two layers fixed:
1. iOS native (SwiftOptimizelyFlutterSdkPlugin.swift)
- Added mainThreadResult() private helper that wraps FlutterResult
to always dispatch on the main thread (no-op if already on main)
- Applied once in handle() so every current and future method handler
is protected automatically — no per-handler changes needed
2. Dart layer (OptimizelyClientWrapper + OptimizelyUserContext)
- Added _invoke() helper that wraps every MethodChannel.invokeMethod
call with a null guard and PlatformException catch
- Null response returns {success:false} instead of crashing via
Map.from(null) → TypeError
- PlatformException is caught and returned as {success:false}
- All 18 call sites in OptimizelyClientWrapper and 14 in
OptimizelyUserContext now route through _invoke
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ore entries Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…branches Covers the two previously-uncovered error branches in the iOS 16 fix: - Native returns null → success:false (no TypeError) - Native throws PlatformException → success:false (no unhandled exception) Tests both OptimizelyClientWrapper and OptimizelyUserContext. Overall unit test coverage: 85.9% → 87.3% Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…roid) Mirrors the iOS mainThreadResult() fix. Added safeResult() wrapper in onMethodCall() that dispatches success/error/notImplemented to the Android main thread via Handler(Looper.getMainLooper()) when called from a background thread. Applied once so all current and future handlers are automatically protected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
muzahidul-opti
changed the title
Flutter 3.0.5 (used in CI) exposes instance as nullable — use ?. pattern consistent with existing test suite to fix compilation failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a threading violation where
FlutterResult/MethodChannel.Resultwas being called from a background thread on both iOS and Android, causing DartFutures to hang or crash with aTypeError.iOS fix (
SwiftOptimizelyFlutterSdkPlugin.swift)Added a
mainThreadResult()private helper that wraps everyFlutterResultcallback to ensure it is always dispatched on the main thread viaDispatchQueue.main.async. Applied once inhandle()so all 25+ method handlers (current and future) are automatically protected.Android fix (
OptimizelyFlutterSdkPlugin.java)Added a
safeResult()private helper that wrapsMethodChannel.Resultand dispatchessuccess,error, andnotImplementedto the main thread viaHandler(Looper.getMainLooper()). Applied once inonMethodCall()so all 28 handlers (current and future) are automatically protected.Dart fix — wrapper (
optimizely_client_wrapper.dart)Added
_invoke()static helper replacing all 18 rawMap<String, dynamic>.from(await _channel.invokeMethod(...))call sites. Handlesnullreturns andPlatformExceptiongracefully, preventingTypeErrorcrashes.Dart fix — user context (
optimizely_user_context.dart)Added identical
_invoke()instance helper replacing all 14 rawinvokeMethodcall sites.Unit tests (
test/invoke_safety_test.dart)17 new tests covering the two previously-uncovered error branches in
_invoke()— null return andPlatformException— for bothOptimizelyClientWrapperandOptimizelyUserContext. Overall unit test coverage: 85.9% → 87.3%.Root Causes
OptimizelyClient.start()(iOS) and async SDK callbacks (Android) fire on background threads →FlutterResult/Resultcalled from wrong thread → response may be dropped or cause undefined behaviour → DartFuturehangs forever.null→Map<String, dynamic>.from(null)→TypeErrorcrash in Dart.Platform Comparison
mainThreadResult(_ result:) -> FlutterResultsafeResult(Result result) -> ResultThread.isMainThreadLooper.myLooper() == Looper.getMainLooper()DispatchQueue.main.async { result(value) }new Handler(Looper.getMainLooper()).post(...)handle()onMethodCall()Test plan
flutter test— all 140 tests pass, coverage 87.3%flutter analyze— no new warningsflutter test integration_test/tests/ios16_threading_regression.dart -d <ios-16-simulator-id>🤖 Generated with Claude Code